-
-
Notifications
You must be signed in to change notification settings - Fork 348
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Move CI jobs for draft PRs to contributor's personal forks of Mill #3543
base: main
Are you sure you want to change the base?
Conversation
So IIUC, PR opened against Mill repository automatically run on Mill CI or the PR creator CI account, depending on the draft status of the PR? Do PRs need to be branches in the users fork or in the Mill repo or are both options supported? Also, if PRs are branches in the users forked repo, don't they already run in their CI? |
Currently all PRs run in the com-lihaoyi/mill github actions. We previously disabled all branch-push triggers except for The current proposed change should work the same for both external PRs as wel as PRs from com-lihaoyi/mill's own branches; in both cases, the choice of trigger depends on whether the PR is a draft or not. Except for internal PRs they both end up running in the same place |
So, this change is all about not running draft PR events but run all push events in any branch instead. It looks to me, that we now run all commits to non-draft PRs, pushed to |
I started to open PRs from the main repository, since this allows scenarios like opening PRs against another PR branch, which gets autoamtically rebased to |
Yes I believe you are correct that as of now this change means non-draft PRs from the main repository will run CI twice. Let me see if I can tweak the github actions conditionals to mitigate that |
This PR aims to reduce the bottleneck of contributors waiting for CI jobs on the shared Mill repo by moving CI for draft/WIP pull requests to each contributor's own fork, each of which has a separate concurrency budget (~20 concurrent workers) from the
com-lihaoyi
organization.Once a PR is marked ready-for-review,
com-lihaoyi
CI begins, so the author effectively has a choice of where they want to run their CI workloads. Mill's main PR validation CI jobs are portable, do not need any special secrets or environment, and can run just fine in contributor's GH Actions environmentsTo do this:
Made the upstream
build-linux
/build-windows
/test-docs
CI jobs filter ongithub.event.pull_request.draft == false
, andRemoved the
branches: - main
filter on therun-tests
jobs so they begin running in contributor GH Actions when commits are pushed to a branchAdd a new
Draft CI
workflow that pushes a github status linking to the PR's branch in the contributor's fork, as an easy way to see the CI history for the PR there. This needs to run onpull_request_target
rather than justpull_request
to have the suitable permissions to update statusesSome subtleties around handling
ready_for_review
events such that when a contributor marks a PR as ready for review, CI is re-triggered and handled appropriately without needing to subsequently push a new commit.Substituted
github.head_ref
withgithub.ref_name
in theconcurrency.group
key since the former only works on PR events while the latter also works on push eventsThis is a bit of an unusual setup AFAIK for Github Actions, but I've tested it manually a bunch and it seems to work, and should mitigate the bottleneck of multiple people updating their WIP PRs and having everyone blocked waiting on everyone else. Worst come to worst, if you mark a PR as
ready_for_review
it falls back to the old behavior, but this setup gives you an option to run draft PR validation on your own fork's GH Actions where it won't get blocked by other people's PRs. This is something you can actually already do today, but it requires some tedious fiddling that PR makes seamlessTested on #3540 via
git commit -am . --allow-empty && git commit -am . --allow-empty && git push origin head && git push upstream head~1:test-non-draft && git commit -am . --allow-empty && git push origin head
, and targeting this PR against thetest-non-draft
branch in the Mill repo